Test under Java 26 - #773
Conversation
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds Java 26 support to the Makefile, Azure Pipelines, CircleCI, job matrices, and Docker images. It updates Gradle to 9.6.1 in plus images, adjusts RockyLinux Java paths, and adds process ownership and cleanup handling in Chicory, Session, and TraceSelect. The changelog documents Java 26 support and Java 8 removal. Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.circleci/config.yml:
- Around line 1048-1051: The workflow references 16 JDK26 job names (e.g.,
quick-txt-diff-ubuntu-jdk26, nonquick-txt-diff-ubuntu-jdk26,
non-txt-diff-ubuntu-jdk26, misc-ubuntu-jdk26, kvasir-ubuntu-jdk26,
typecheck-latest-part1-ubuntu-jdk26 ... typecheck-bundled-part3-ubuntu-jdk26,
plus the rockylinux variants) that are not defined under the top-level jobs:
map; add corresponding job stanzas for each of these 16 names in the jobs:
section by copying their JDK25 counterparts (e.g., quick-txt-diff-ubuntu-jdk25,
typecheck-bundled-part2-ubuntu-jdk25, quick-txt-diff-rockylinux-jdk25, etc.) and
update JDK-specific settings (Docker image tag, environment variables, any
matrix or executor names) from jdk25 to jdk26 so the workflow references match
defined jobs.
In `@scripts/Dockerfile-rockylinux-jdk26-plus`:
- Around line 68-72: The RUN layer currently calls dnf config-manager
--set-enabled crb before installing dnf-plugins-core, which can fail on the base
image; change the RUN instruction so dnf-plugins-core is installed first
(install dnf-plugins-core in the same or a prior RUN) and then run dnf
config-manager --set-enabled crb, ensuring the plugin is available when running
the config-manager command.
- Around line 91-96: Replace the unpinned Astral installer URL used in the RUN
step with a version-pinned URL and propagate this change across all "-plus"
Dockerfile variants; specifically, introduce a UV_VERSION (e.g., 0.10.11) and
use the installer path https://astral.sh/uv/<UV_VERSION>/install.sh in the RUN
that currently contains "wget -qO- https://astral.sh/uv/install.sh | sh",
updating the RUN block that sets up uv and preserving the subsequent chmod and
ENV PATH lines so builds are reproducible across the Ubuntu and RockyLinux
"-plus" Dockerfiles.
In `@scripts/Dockerfile-ubuntu-jdk26-plus`:
- Around line 87-100: Pin versions and add checksum verification: change the two
pipx installs (PIPX_HOME=/opt/pipx PIPX_BIN_DIR=/usr/local/bin pipx install mypy
and pipx install ruff) to install specific released versions (e.g., pipx install
mypy==<version> and pipx install ruff==<version>) and update any related PIPX_*
env usage accordingly; replace the wget | sh for uv (wget -qO-
https://astral.sh/uv/install.sh | sh) with downloading a specific uv release
artifact (use a fixed release URL), verify its checksum (e.g., sha256) before
executing, and invoke the installer from the verified file so the Dockerfile
uses pinned, reproducible tooling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e3bead74-da5d-4e37-a9f5-5783cf7eab88
📒 Files selected for processing (12)
.azure/azure-pipelines.yml.azure/azure-pipelines.yml.m4.azure/jobs.m4.circleci/config.yml.circleci/config.yml.m4doc/CHANGELOG.mdjava/Makefilescripts/Dockerfile-READMEscripts/Dockerfile-rockylinux-jdk26scripts/Dockerfile-rockylinux-jdk26-plusscripts/Dockerfile-ubuntu-jdk26scripts/Dockerfile-ubuntu-jdk26-plus
There was a problem hiding this comment.
♻️ Duplicate comments (1)
java/Makefile (1)
54-58:⚠️ Potential issue | 🟠 Major
JAVA26_HOMEis still bypassed or fails open depending onJAVA_HOME.This is the same root cause as the earlier JDK 26 toolchain-selection comment:
JAVA25_HOME := ${JAVA_HOME}is expanded before the laterJAVA_HOME ?= ...autodetection block. IfJAVA_HOMEis exported, Line 75 always wins and the newJAVA26_HOMEfallback added forscripts/Dockerfile-ubuntu-jdk26Line 51 /scripts/Dockerfile-rockylinux-jdk26Line 40 is never used. IfJAVA_HOMEis not exported,JAVA25_HOMEbecomes empty and Line 82 aborts beforeJAVA_HOMEcan be derived fromwhich javac. That leaves JDK 26 builds dependent on ambient env vars instead of the selected toolchain.💡 Possible fix
ifeq ($(shell test ${JAVA_RELEASE_NUMBER} -ge 25; echo $$?),0) JAVA25 := 1 - ifndef JAVA25_HOME - JAVA25_HOME := ${JAVA_HOME} + ifeq ($(shell test ${JAVA_RELEASE_NUMBER} -eq 25; echo $$?),0) + JAVA25_HOME ?= ${JAVA_HOME} endif endif ... ifeq (${JAVA25}, 1) ifdef JAVA25_HOME JAVAC_TARGET_FLAGS ?= --source 25 --target 25 --system ${JAVA25_HOME} else ifdef JAVA26_HOME JAVAC_TARGET_FLAGS ?= --source 25 --target 25 --system ${JAVA26_HOME} else - $(error JAVA25_HOME and JAVA26_HOME are not set; JAVA_HOME=${JAVA_HOME}) + JAVAC_TARGET_FLAGS ?= --source 25 --target 25 --system ${JAVA_HOME} endif endif else#!/bin/bash set -euo pipefail echo "Relevant logic from java/Makefile:" sed -n '54,83p' java/Makefile echo echo "Later JAVA_HOME autodetection:" sed -n '132,137p' java/Makefile cat >/tmp/java26-home-repro.mk <<'EOF' JAVA25 := 1 ifndef JAVA25_HOME JAVA25_HOME := ${JAVA_HOME} endif ifeq (${JAVA25}, 1) ifdef JAVA25_HOME TARGET := ${JAVA25_HOME} else ifdef JAVA26_HOME TARGET := ${JAVA26_HOME} else $(error JAVA25_HOME and JAVA26_HOME are not set; JAVA_HOME=${JAVA_HOME}) endif endif endif JAVA_HOME ?= /detected/later/from-path print: ; `@printf` 'TARGET=%s\n' '$(TARGET)' EOF echo echo "Case 1: exported JAVA_HOME keeps the JAVA26_HOME branch unreachable:" JAVA_HOME=/env/jdk26 JAVA26_HOME=/explicit/jdk26 make -f /tmp/java26-home-repro.mk print echo echo "Case 2: with neither variable exported, the file errors before the later JAVA_HOME default is visible:" if make -f /tmp/java26-home-repro.mk print >/tmp/java26-home-repro.err 2>&1; then echo "unexpected success" exit 1 else sed -n '1,5p' /tmp/java26-home-repro.err fiAlso applies to: 74-83
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@java/Makefile` around lines 54 - 58, The Makefile sets JAVA25_HOME from JAVA_HOME too early which prevents the later JAVA_HOME autodetection and blocks the JAVA26_HOME fallback; update the logic so JAVA_HOME is determined first or avoid early unconditional expansion—remove or change "JAVA25_HOME := ${JAVA_HOME}" to a conditional assignment (e.g. use "?="), or better: do not set JAVA25_HOME there and instead compute TARGET after the "JAVA_HOME ?= ..." autodetection using the chain (ifdef JAVA25_HOME -> TARGET=JAVA25_HOME else ifdef JAVA26_HOME -> TARGET=JAVA26_HOME else error), ensuring JAVA25_HOME, JAVA26_HOME and JAVA_HOME references (and TARGET) resolve correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@java/Makefile`:
- Around line 54-58: The Makefile sets JAVA25_HOME from JAVA_HOME too early
which prevents the later JAVA_HOME autodetection and blocks the JAVA26_HOME
fallback; update the logic so JAVA_HOME is determined first or avoid early
unconditional expansion—remove or change "JAVA25_HOME := ${JAVA_HOME}" to a
conditional assignment (e.g. use "?="), or better: do not set JAVA25_HOME there
and instead compute TARGET after the "JAVA_HOME ?= ..." autodetection using the
chain (ifdef JAVA25_HOME -> TARGET=JAVA25_HOME else ifdef JAVA26_HOME ->
TARGET=JAVA26_HOME else error), ensuring JAVA25_HOME, JAVA26_HOME and JAVA_HOME
references (and TARGET) resolve correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 938f3dbd-c45c-466c-b468-706d9bbe43c7
📒 Files selected for processing (1)
java/Makefile
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
java/Makefile (1)
59-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Java 26 build failure caused by
JAVAC_TARGET_FLAGSrequiringJAVA25_HOME/JAVA26_HOMEbeforeJAVA_HOMEis detected
- With
JAVA_RELEASE_NUMBER >= 25,JAVA25 := 1, so theJAVAC_TARGET_FLAGSblock always uses--system ${JAVA25_HOME}or--system ${JAVA26_HOME}and errors out if both are unset.JAVA25_HOMEis only seeded whenJAVA_RELEASE_NUMBER == 25, whileJAVA_HOME ?= ...detection happens later in the file, so local JDK 26 builds can fail unlessJAVA26_HOME(orJAVA25_HOME) is pre-exported (CI masks this viaJAVA26_HOME).- Move the
JAVA_HOMEdiscovery block above theJAVAC_TARGET_FLAGScomputation (or otherwise deriveJAVA26_HOMEfrom the detectedjavac/JAVA_HOMEbefore the error path).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 59 - 66, The build fails for Java 26 because JAVAC_TARGET_FLAGS chooses --system ${JAVA25_HOME} or ${JAVA26_HOME} before JAVA_HOME is discovered; move the JAVA_HOME discovery / javac detection logic earlier so JAVA25_HOME/JAVA26_HOME can be derived before computing JAVAC_TARGET_FLAGS, or alternatively detect javac/JAVA_HOME and set JAVA26_HOME (and JAVA25_HOME fallback) immediately after computing JAVA_RELEASE_NUMBER and before the JAVAC_TARGET_FLAGS block so the flags use valid paths; update references to JAVA_RELEASE_NUMBER, JAVA25_HOME, JAVA26_HOME, JAVAC_TARGET_FLAGS and JAVA_HOME accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@java/Makefile`:
- Around line 59-66: The build fails for Java 26 because JAVAC_TARGET_FLAGS
chooses --system ${JAVA25_HOME} or ${JAVA26_HOME} before JAVA_HOME is
discovered; move the JAVA_HOME discovery / javac detection logic earlier so
JAVA25_HOME/JAVA26_HOME can be derived before computing JAVAC_TARGET_FLAGS, or
alternatively detect javac/JAVA_HOME and set JAVA26_HOME (and JAVA25_HOME
fallback) immediately after computing JAVA_RELEASE_NUMBER and before the
JAVAC_TARGET_FLAGS block so the flags use valid paths; update references to
JAVA_RELEASE_NUMBER, JAVA25_HOME, JAVA26_HOME, JAVAC_TARGET_FLAGS and JAVA_HOME
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 64face08-f189-425d-9101-e44f522bcaae
📒 Files selected for processing (7)
.azure/azure-pipelines.yml.azure/jobs.m4.circleci/config.yml.circleci/config.yml.m4doc/CHANGELOG.mdjava/Makefilejava/daikon/chicory/Instrument24.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@java/Makefile`:
- Around line 26-35: The outer Darwin conditional ifeq ($(shell uname), Darwin)
currently remains open until much later, which wrongly scopes
JAVA_RELEASE_NUMBER and related setup to non-Darwin systems; close that outer
conditional immediately after setting JAVA_HOME on Darwin by inserting an endif
right after the Darwin branch (right after the line with JAVA_HOME ?=
$(/usr/libexec/java_home)) so that the subsequent non-Darwin javac detection and
the JAVA_RELEASE_NUMBER/JAVA_TARGET_FLAGS logic apply to all platforms; ensure
the symbols referenced (ifeq ($(shell uname), Darwin), JAVA_HOME,
JAVA_RELEASE_NUMBER, JAVA_TARGET_FLAGS) are no longer nested under the Darwin
block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 41a050c1-a804-4669-9166-76869c4ac06c
📒 Files selected for processing (1)
java/Makefile
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
java/Makefile (3)
26-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Darwin
JAVA_HOMEdetection syntax injava/Makefile(line 27).
JAVA_HOME ?= $(/usr/libexec/java_home)is GNU Make variable expansion (not command execution), so it will expand to empty on macOS. Use$(shell ...)to run the command and capture its output.Suggested fix
ifeq ($(shell uname), Darwin) - JAVA_HOME ?= $(/usr/libexec/java_home) + JAVA_HOME ?= $(shell /usr/libexec/java_home) else🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 26 - 27, The Darwin branch in the Makefile uses GNU Make variable expansion instead of executing the macOS helper, so change the JAVA_HOME assignment in the ifeq ($(shell uname), Darwin) block to execute /usr/libexec/java_home and capture its output (i.e., replace the current JAVA_HOME ?= $(/usr/libexec/java_home) line with one that uses $(shell /usr/libexec/java_home) so JAVA_HOME is set correctly).
39-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDerive
JAVA_RELEASE_NUMBERfrom the selected compiler (JAVAC/JAVA_HOME), not PATHjavac.
java/MakefileloadsMakefile.userfirst (-include Makefile.user), butJAVA_VERSION_STRING_WITH_EA/JAVA_VERSION_STRINGare still computed via$(shell javac -version ...)(hardcoded). The actual compile uses${JAVAC}(JAVAC_COMMAND := ${JAVAC} ...), so overridingJAVAC/JAVA_HOMEcan desync the computedJAVA_RELEASE_NUMBERand therefore the derivedJAVAC_TARGET_FLAGS(e.g., falling back to-source 8 -target 8based on whateverjavacis on PATH).Suggested fix
+JAVAC_VERSION_CMD := $(if ${JAVA_HOME},${JAVA_HOME}/bin/javac,$(firstword ${JAVAC})) -JAVA_VERSION_STRING_WITH_EA := $(shell javac -version 2>&1 | head -1 | cut "-d " -f2) -JAVA_VERSION_STRING := $(shell javac -version 2>&1 | head -1 | cut "-d " -f2 | sed 's/-ea//') +JAVA_VERSION_STRING_WITH_EA := $(shell ${JAVAC_VERSION_CMD} -version 2>&1 | head -1 | cut "-d " -f2) +JAVA_VERSION_STRING := $(shell ${JAVAC_VERSION_CMD} -version 2>&1 | head -1 | cut "-d " -f2 | sed 's/-ea//')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 39 - 40, The version variables JAVA_VERSION_STRING_WITH_EA and JAVA_VERSION_STRING are being derived from the PATH `javac` instead of the actual compiler chosen by the Makefile (JAVAC/JAVA_HOME), causing JAVA_RELEASE_NUMBER and JACAC_TARGET_FLAGS to drift; change the shell invocations to call the selected compiler variable (use ${JAVAC} or ${JAVA_HOME}/bin/javac if you prefer) when computing JAVA_VERSION_STRING_WITH_EA and JAVA_VERSION_STRING (e.g., run `${JAVAC} -version 2>&1 | ...`) after Makefile.user is included so overrides take effect, and ensure all later logic that computes JAVA_RELEASE_NUMBER and sets JAVAC_TARGET_FLAGS uses those updated variables instead of the hardcoded `javac`.
76-98:⚠️ Potential issue | 🟠 Major | ⚡ Quick winJDK 26 local builds can fail unless
JAVA26_HOMEis set (theJAVA_HOMEdetection doesn’t prevent the$(error ...)).When
JAVA_RELEASE_NUMBER >= 26,JAVA25is also enabled, butJAVA25_HOMEis only defaulted forJAVA_RELEASE_NUMBER == 25andJAVA26_HOMEis never defaulted. As a result, theJAVAC_TARGET_FLAGSblock errors if neitherJAVA25_HOMEnorJAVA26_HOMEis defined, even thoughJAVA_HOMEis detected earlier.Suggested fix
ifeq ($(shell test ${JAVA_RELEASE_NUMBER} -ge 26; echo $$?),0) JAVA26 := 1 + ifndef JAVA26_HOME + JAVA26_HOME := ${JAVA_HOME} + endif endifTest with a local JDK 26 where only
JAVA_HOMEis set (noJAVA26_HOME).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/Makefile` around lines 76 - 98, The Makefile errors when JAVA25 is enabled for JDK 26 because the JAVAC_TARGET_FLAGS block requires JAVA25_HOME or JAVA26_HOME even though JAVA_HOME may be set; update the logic around JAVA25/JAVA26 handling so that when JAVA_RELEASE_NUMBER >= 26 you default JAVA26_HOME from JAVA_HOME if JAVA26_HOME is not defined (or accept JAVA_HOME as a valid fallback) before emitting the $(error), and then set JAVAC_TARGET_FLAGS using the resolved JAVA25_HOME or JAVA26_HOME; specifically adjust the conditional that references JAVA25, JAVA25_HOME, JAVA26_HOME, and JAVAC_TARGET_FLAGS to prefer existing JAVA*_HOME values and fallback to JAVA_HOME so local JDK 26 builds succeed without an explicit JAVA26_HOME.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@java/Makefile`:
- Around line 26-27: The Darwin branch in the Makefile uses GNU Make variable
expansion instead of executing the macOS helper, so change the JAVA_HOME
assignment in the ifeq ($(shell uname), Darwin) block to execute
/usr/libexec/java_home and capture its output (i.e., replace the current
JAVA_HOME ?= $(/usr/libexec/java_home) line with one that uses $(shell
/usr/libexec/java_home) so JAVA_HOME is set correctly).
- Around line 39-40: The version variables JAVA_VERSION_STRING_WITH_EA and
JAVA_VERSION_STRING are being derived from the PATH `javac` instead of the
actual compiler chosen by the Makefile (JAVAC/JAVA_HOME), causing
JAVA_RELEASE_NUMBER and JACAC_TARGET_FLAGS to drift; change the shell
invocations to call the selected compiler variable (use ${JAVAC} or
${JAVA_HOME}/bin/javac if you prefer) when computing JAVA_VERSION_STRING_WITH_EA
and JAVA_VERSION_STRING (e.g., run `${JAVAC} -version 2>&1 | ...`) after
Makefile.user is included so overrides take effect, and ensure all later logic
that computes JAVA_RELEASE_NUMBER and sets JAVAC_TARGET_FLAGS uses those updated
variables instead of the hardcoded `javac`.
- Around line 76-98: The Makefile errors when JAVA25 is enabled for JDK 26
because the JAVAC_TARGET_FLAGS block requires JAVA25_HOME or JAVA26_HOME even
though JAVA_HOME may be set; update the logic around JAVA25/JAVA26 handling so
that when JAVA_RELEASE_NUMBER >= 26 you default JAVA26_HOME from JAVA_HOME if
JAVA26_HOME is not defined (or accept JAVA_HOME as a valid fallback) before
emitting the $(error), and then set JAVAC_TARGET_FLAGS using the resolved
JAVA25_HOME or JAVA26_HOME; specifically adjust the conditional that references
JAVA25, JAVA25_HOME, JAVA26_HOME, and JAVAC_TARGET_FLAGS to prefer existing
JAVA*_HOME values and fallback to JAVA_HOME so local JDK 26 builds succeed
without an explicit JAVA26_HOME.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4e8c1efc-583e-423d-bb2b-e3f9f76281a0
📒 Files selected for processing (1)
java/Makefile
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.azure/azure-pipelines.yml.m4 (1)
23-23: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winInconsistent wildcard pattern in comment.
The dependency rule comment uses
*_jdk{version}for JDK 8, 11, 17, and 21, but omits the asterisk prefix for JDK 26. For consistency and clarity, change_jdk26to*_jdk26.📝 Proposed fix
- # * Anything *_jdk8 or *_jdk11 or *_jdk17 or *_jdk21 or _jdk26 depends on *_jdk25. + # * Anything *_jdk8 or *_jdk11 or *_jdk17 or *_jdk21 or *_jdk26 depends on *_jdk25.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.azure/azure-pipelines.yml.m4 at line 23, Update the comment line that lists dependency patterns so the wildcard is consistent: change `_jdk26` to `*_jdk26` so the comment reads like the other patterns (e.g., `*_jdk8`, `*_jdk11`, `*_jdk17`, `*_jdk21`, `*_jdk26`)—locate the line containing the patterns `*_jdk8 or *_jdk11 or *_jdk17 or *_jdk21 or _jdk26` and replace the trailing `_jdk26` with `*_jdk26`..circleci/config.yml (1)
97-114:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd missing pipeline parameter to JDK 26 Docker image reference.
All other JDK versions (11, 17, 21, 25) include
<< pipeline.parameters.testing-suffix >>in their Docker image references (see lines 23, 42, 61, 80), but this JDK 26 job omits it. This inconsistency prevents JDK 26 jobs from using the testing-suffix parameter for pre-release image testing.🔧 Proposed fix
quick-txt-diff-ubuntu-jdk26: docker: - - image: mdernst/daikon-ubuntu-jdk26 + - image: mdernst/daikon-ubuntu-jdk26<< pipeline.parameters.testing-suffix >> resource_class: largeThis same fix must be applied to all 16 JDK 26 job definitions (Ubuntu: lines 99, 194, 285, 388, 452, 793, 814, 835, 856, 877, 898; RockyLinux: lines 958, 1015, 1070, 1131, 1195).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.circleci/config.yml around lines 97 - 114, The quick-txt-diff-ubuntu-jdk26 job uses the docker image reference "mdernst/daikon-ubuntu-jdk26" but omits the pipeline parameter; update the docker image string for the quick-txt-diff-ubuntu-jdk26 job (and the other JDK26 job definitions) to append << pipeline.parameters.testing-suffix >> to the image name so it matches the other JDK jobs, ensuring all 16 JDK 26 job definitions (e.g., the jobs named quick-txt-diff-ubuntu-jdk26 and the RockyLinux JDK26 equivalents) include the testing-suffix parameter in their docker.image entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.azure/azure-pipelines.yml.m4:
- Line 23: Update the comment line that lists dependency patterns so the
wildcard is consistent: change `_jdk26` to `*_jdk26` so the comment reads like
the other patterns (e.g., `*_jdk8`, `*_jdk11`, `*_jdk17`, `*_jdk21`,
`*_jdk26`)—locate the line containing the patterns `*_jdk8 or *_jdk11 or *_jdk17
or *_jdk21 or _jdk26` and replace the trailing `_jdk26` with `*_jdk26`.
In @.circleci/config.yml:
- Around line 97-114: The quick-txt-diff-ubuntu-jdk26 job uses the docker image
reference "mdernst/daikon-ubuntu-jdk26" but omits the pipeline parameter; update
the docker image string for the quick-txt-diff-ubuntu-jdk26 job (and the other
JDK26 job definitions) to append << pipeline.parameters.testing-suffix >> to the
image name so it matches the other JDK jobs, ensuring all 16 JDK 26 job
definitions (e.g., the jobs named quick-txt-diff-ubuntu-jdk26 and the RockyLinux
JDK26 equivalents) include the testing-suffix parameter in their docker.image
entries.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e5b44eb-cf73-409c-9811-9449737446b8
📒 Files selected for processing (4)
.azure/azure-pipelines.yml.azure/azure-pipelines.yml.m4.circleci/config.yml.circleci/config.yml.m4
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/Dockerfile-rockylinux-jdk11-plus`:
- Around line 58-63: The Dockerfile has duplicate Gradle install logic, so
consolidate the two Gradle setup blocks into a single one and remove the
redundant copy. Keep the shared download/unzip/rm sequence and the PATH update
only once in the Dockerfile-rockylinux-jdk11-plus so it matches the pattern used
by the sibling Dockerfiles and avoids duplicate layers. Reference the Gradle
install sections around the existing RUN/ENV block and the later repeated block
when making the cleanup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f4d9fbc6-8f7a-4d30-9d7c-f4ec4ee13fcc
📒 Files selected for processing (11)
scripts/Dockerfile-rockylinux-jdk11-plusscripts/Dockerfile-rockylinux-jdk17-plusscripts/Dockerfile-rockylinux-jdk21-plusscripts/Dockerfile-rockylinux-jdk25-plusscripts/Dockerfile-rockylinux-jdk26scripts/Dockerfile-rockylinux-jdk26-plusscripts/Dockerfile-ubuntu-jdk11-plusscripts/Dockerfile-ubuntu-jdk17-plusscripts/Dockerfile-ubuntu-jdk21-plusscripts/Dockerfile-ubuntu-jdk25-plusscripts/Dockerfile-ubuntu-jdk26-plus
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@java/daikon/Chicory.java`:
- Line 35: Resolve the lifecycle contract on Chicory by either restoring an
active close() implementation or removing the related annotations until disposal
is supported. Prefer implementing close() to dispose of the `@Owning` daikon_proc
using close() on Java 26 and destroy() on older Java versions, then ensure the
Chicory instance is closed on all relevant paths, including main().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f73fb9bf-1823-41a7-aa0b-bdbd53841903
📒 Files selected for processing (3)
java/daikon/Chicory.javajava/daikon/simplify/Session.javajava/daikon/tools/TraceSelect.java
There was a problem hiding this comment.
♻️ Duplicate comments (1)
java/daikon/Chicory.java (1)
35-35: 🩺 Stability & Availability | 🟠 Major
@InheritableMustCall("close")declared butclose()is still commented out.This was flagged in a prior review and remains unaddressed. The must-call checker will flag the
Chicoryinstance created inmain()(line 227) as missing its requiredclose()call, and the@Owningondaikon_proc(line 188) has no active disposal path.Additional issues in the commented-out stub that will surface when uncommented:
@Overridewill fail to compile —Chicorydoesn't implementAutoCloseableorCloseable.daikon_proc.close()will NPE when neither--daikonnor--daikon-onlineis specified (field stays null).Also applies to: 668-672
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/daikon/Chicory.java` at line 35, Implement an active close lifecycle for Chicory: make the class implement AutoCloseable or Closeable, uncomment or add close() to dispose daikon_proc safely when it is non-null, and ensure main() closes the Chicory instance created there on every exit path. Preserve the `@InheritableMustCall`("close") contract and `@Owning` disposal semantics without dereferencing daikon_proc when neither Daikon mode is enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@java/daikon/Chicory.java`:
- Line 35: Implement an active close lifecycle for Chicory: make the class
implement AutoCloseable or Closeable, uncomment or add close() to dispose
daikon_proc safely when it is non-null, and ensure main() closes the Chicory
instance created there on every exit path. Preserve the
`@InheritableMustCall`("close") contract and `@Owning` disposal semantics without
dereferencing daikon_proc when neither Daikon mode is enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f4c76887-edb6-46cf-9ed9-159bcb363ffd
📒 Files selected for processing (1)
java/daikon/Chicory.java
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
java/daikon/simplify/Session.java (1)
80-80: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winConstructor catch block doesn't clean up the newly
@Owningprocess.The
@Owningannotation declares thatSessionis responsible for disposing ofprocess. The constructor assignsprocessat line 111, but the catch block (lines 143–152) only closestrace_file— it does not destroyprocess. If construction fails after line 111 (e.g.,sendLinethrows, or the prompt assertion at line 141 fails), the Simplify process leaks becauseclose()is never called on a partially-constructedSession.This mirrors the existing
trace_filecleanup pattern already in the catch block —processneeds the same treatment.🔒 Proposed fix: destroy process in constructor catch block
} catch (Exception | AssertionError e) { + if (process != null) { + process.destroyForcibly(); + } if (trace_file != null) { try { trace_file.close(); } catch (Exception closeException) { e.addSuppressed(closeException); } } throw new SimplifyError(e);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@java/daikon/simplify/Session.java` at line 80, Update the Session constructor’s catch block to destroy the newly assigned process before propagating the construction failure, mirroring the existing trace_file cleanup. Ensure cleanup covers failures after process assignment, including sendLine or prompt assertion failures, without changing normal construction behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@java/daikon/simplify/Session.java`:
- Line 80: Update the Session constructor’s catch block to destroy the newly
assigned process before propagating the construction failure, mirroring the
existing trace_file cleanup. Ensure cleanup covers failures after process
assignment, including sendLine or prompt assertion failures, without changing
normal construction behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 260cb236-ccf6-4f04-a993-c016a18fd00d
📒 Files selected for processing (3)
java/daikon/Chicory.javajava/daikon/simplify/Session.javajava/daikon/tools/TraceSelect.java
2 participants
No description provided.